Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CNI Acceptance test on GKE #1446

Merged
merged 8 commits into from
Aug 25, 2022
Merged

CNI Acceptance test on GKE #1446

merged 8 commits into from
Aug 25, 2022

Conversation

curtbushko
Copy link
Contributor

@curtbushko curtbushko commented Aug 25, 2022

This PR builds on top of the CNI acceptance tests on Kind PR to get things running on GKE.

GKE is the only cloud platform that has a different requirement for connectInject.cni.cniBinDir and does not use the default. The consul-cni binary needs to be installed to /home/kubernetes/bin.

Changes proposed in this PR:

  • Added a -use-gke flag to the acceptance test command so that the flag can be passed through to the tests
  • If both, -enable-cni and -use-gke are passed through then the cniBinDir gets set for helm values.
  • GKE would not install the CNI plugin unless the use security policy was set.
  • GKE was throwing an error saying that allowPrivilegeEscalation: true needed to be set if privileged: true. Once set, tests passed.

How I've tested this PR:

  • ran the acceptance tests in pipelines

How I expect reviewers to test this PR:

👀

Checklist:

  • Tests added
  • CHANGELOG entry added

    HashiCorp engineers only, community PRs should not add a changelog entry.
    Entries should use present tense (e.g. Add support for...)

@curtbushko curtbushko self-assigned this Aug 25, 2022
@@ -1089,6 +1158,11 @@ workflows:
# requires:
# - cleanup-gcp-resources
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will clean this all up before merging

Copy link
Contributor

@thisisnotashwin thisisnotashwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:chef-kiss:

- run: mkdir -p $TEST_RESULTS

- run-acceptance-tests:
additional-flags: -kubeconfig="$primary_kubeconfig" -secondary-kubeconfig="$secondary_kubeconfig" -enable-pod-security-policies -enable-transparent-proxy -enable-cni -enable-gke
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could this flag be called -use-gke similar to -use-kind?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit addressed. 😺

Copy link
Contributor

@ishustava ishustava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just one tiny comment

@curtbushko curtbushko force-pushed the curtbushko/cni-acceptance-gke branch 2 times, most recently from c72ac62 to de02979 Compare August 25, 2022 17:51
Base automatically changed from curtbushko/cni-acceptance-kind to cni August 25, 2022 19:23
@curtbushko curtbushko force-pushed the curtbushko/cni-acceptance-gke branch from de02979 to 83c8100 Compare August 25, 2022 19:38
-  Added a new make target for installing Calico CNI in Kind. The target installs Calico CNI using config files located under /acceptance tests
- Added a helper make target for setting up local Kind with Calico just in case anyone wants to run it.
- Added a kind.config for setting up the Kind cluster
- Added an -enable-cni flag to the acceptance test config so that it can be passed through from CircleCI
-  Added a nightly circleci job for running the CNI kind tests
-  I had missed a bats test for the connect-inject template that I merged in a previous PR
…connectInject.cni.enabled from several files as the framework will set this when the flag is passed in
- Added a -ue-gke flag to the acceptance test command so that the flag can be passed through to the tests
-  If both, -enable-cni and -ue-gke are passed through then the cniBinDir gets set for helm values.
- GKE would not install the CNI plugin unless the use security policy was set.
- GKE was throwing an error saying that allowPrivilegeEscalation: true needed to be set if privileged: true. Once set, tests passed.
@curtbushko curtbushko force-pushed the curtbushko/cni-acceptance-gke branch from 83c8100 to aeb3f2e Compare August 25, 2022 22:02
@curtbushko curtbushko merged commit 0de4b29 into cni Aug 25, 2022
@curtbushko curtbushko deleted the curtbushko/cni-acceptance-gke branch August 25, 2022 22:04
curtbushko added a commit that referenced this pull request Aug 26, 2022
- Added a -use-gke flag to the acceptance test command so that the flag can be passed through to the tests
- If both, -enable-cni and -use-gke are passed through then the cniBinDir gets set for helm values.
- GKE would not install the CNI plugin unless the use security policy was set.
- GKE was throwing an error saying that allowPrivilegeEscalation: true needed to be set if privileged: true. Once set, tests passed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants